Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support placeholder and selectable-overloads #190

Merged

Conversation

ikatyang
Copy link
Member

@ikatyang ikatyang commented Aug 13, 2017

Fixes #173
Fixes #181
Fixes #120

Migrate from ikatyang/types-ramda v0.24.3, see discussion in #173.

To confirm:

  • travis: is typescript@next still needed?
  • travis: need to set a GH_TOKEN on travis to deploy dist-* branches automatically.
  • github: need to add orphan branches to deploy branches.
    • branch names: dist, dist-simple, dist-selectable and dist-placeholder
    • git checkout --orphan $branch-name
  • readme: I have no idea how to modify the roadmap. (seems no need to modify?)
  • something I forgot?

Future:

  • add scripts to generate DT compatible definition/test files.

@KiaraGrouwstra
Copy link
Member

The wait was worth it. :)

travis: is typescript@next still needed?

Guess we can just manually upgrade, whichever.

travis: need to set a GH_TOKEN on travis to deploy dist-* branches automatically.

Right, you mentioned that. Can't say I have one but afaik this didn't seem critical.

readme: I have no idea how to modify the roadmap.

Yeah, whatever. I mean, if I can help it things will move toward more granular type-level operations, but I wouldn't have others worry about that part at least. I suppose the github issue list should suffice to keep things current.

branches

I had trouble pushing branches as orphan. If it'd do, it did work for non-orphan versions. If push --force or whatever could help fix that again, then great.

On that topic, @blakeembrey, could you invite @ikatyang into types? I've shifted to working on the higher-level challenges with TS (started a type lib, now making a few compiler PRs) to further get this thing forward, while he's managed to fully rewrite these Ramda typings again.

@KiaraGrouwstra
Copy link
Member

On package.json, just realized a few points from the existing one may have come from expectations of @types (linting config) / DT (typings prop?).

@ikatyang
Copy link
Member Author

ikatyang commented Aug 14, 2017

  • The typings prop (value = index.d.ts) in package.json is not necessary, since it is the default value (index.d.ts).

  • branches: I'm under the impression that it has to use --allow-unrelated-histories to push orphan branches. (normal branch is okay too, but I think orphan branch is clearer)

  • I prefer using yarn instead of npm, is package-lock.json necessary? if so, I'll replace yarn with npm. (the package-lock.json was somehow added in your merge commit)

@KiaraGrouwstra
Copy link
Member

I prefer using yarn instead of npm, is package-lock.json necessary? if so, I'll replace yarn with npm. (the package-lock.json was somehow added in your merge commit)

Sorry, that's fine. I'd felt recent npm simplified things by handling exact version management without the extra dependency, but I don't have a strong preference otherwise; not sure what other considerations might be at play.

--allow-unrelated-histories

Uh, that's part of the pull command huh? Sorry if I'm being dense -- how should I use that here?

@ikatyang
Copy link
Member Author

Sorry, it does not need --allow-unrelated-histories option 😅

BRANCH=dist
git checkout --orphan $BRANCH
git commit --allow-empty -m "init"
git push --set-upstream origin $BRANCH                                                                                   

@KiaraGrouwstra
Copy link
Member

KiaraGrouwstra commented Aug 14, 2017

Done!

Edit: Tell me when I should merge this PR; I presume you want me to after you've added the branches.

@ikatyang
Copy link
Member Author

The last thing is about the GH_TOKEN, if there's no GH_TOKEN, it won't deploy automatically, I can't confirm this since it can only be set on the Travis Settings (Environment Variables, it'll automatically be encrypted):

image

@KiaraGrouwstra
Copy link
Member

KiaraGrouwstra commented Aug 14, 2017

How can I help on this part? So you don't want one from me huh? Does the bot need @types write access?

So we should enter a token in the repo settings huh? Let me try that...
Do you know what privileges it should have?

@ikatyang
Copy link
Member Author

ikatyang commented Aug 14, 2017

It's OK from you, but if that GH_TOKEN somehow exposed, it'll be dangerous since it'll have all your public repo's write permission. My solution is that I created a bot account and add it as a collaborator, and uses its GH_TOKEN, so that it's less dangerous than use our own.

(Just need this repo's write access, but GH_TOKEN can only choose the public_repo permission)

EDIT: (there's no way to generate GH_TOKEN for only one repo, GitHub does not support it.)

@KiaraGrouwstra
Copy link
Member

I wish I could invite you into the org myself, but I fear I'd need to wait for @blakeembrey to respond there. It's good you already made the bot account at least.

If he doesn't respond yet and so far I'm the closest account with a token with access though, might I be able to put a token into Travis? I presume it getting exposed would require them getting hacked or something. I guess the Travis account is yours?

@ikatyang
Copy link
Member Author

Since the GH_TOKEN will put in Environment Variables, it might have some concern of something like crossenv event, but in short term, I think it's okay to use yours temporarily.

I guess the Travis account is yours?

I'm not sure what you mean the Travis account?

@KiaraGrouwstra
Copy link
Member

As in, where can I put this environment variable in? I can't seem to find a page in Github that looks like the one in your screenshot. 😅

@ikatyang
Copy link
Member Author

It's on travis 😅, should be in https://travis-ci.org/types/npm-ramda

image

@KiaraGrouwstra
Copy link
Member

Cool, added it in now! :)

@ikatyang
Copy link
Member Author

@blakeembrey

Thanks for inviting me to the @types org. 😀

Can you invite my bot account @ikatyang-bot to be a collaborator of this repo to deploy generated definitions automatically (just like this)? and set the master branch to be a protected branch so that auto-deploy won't accidentally override it, though it's almost the impossible thing to happen, but it'd be great to prevent it. Sorry for bothering you so much.

@tycho01

I guess we can merge this PR now, and I'll replace the GH_TOKEN with my bot's token (looks like I have no permission to add my bot-account as collaborator to this repo, so I'll use mine if there's no other choice) in few days since Travis needs time to sync permission. (I'll tell you when I replaced the token so that you can disable your token)

@blakeembrey
Copy link
Member

Sure, no problem. What exactly is the bot doing and would it be applicable to the wider organisation? If so, maybe we can find a better approach for you.

@ikatyang
Copy link
Member Author

ikatyang commented Aug 15, 2017

The @ikatyang-bot is just for this repo's deployment: generated four version of the definitions and push them to their own branch: dist, dist-simple, dist-placeholder and dist-selectable.

To clarify, this bot account is just for reducing the risk of Personal Access Token, since GitHub can only generate token for public_repo and can't generate token only for the specific repo. I'm afraid of something like crossenv event, so I'd prefer not to use our own account's token directly.

So that people can download which version they want via something like:

npm install types/npm-ramda#dist
npm install types/npm-ramda#dist-simple
npm install types/npm-ramda#dist-placeholder
npm install types/npm-ramda#dist-selectable

You can see my current deployment in my repo for example.

EDIT: can you reduce the permission for my bot account to only access this repo (collaborator)? 😅 thanks.

@KiaraGrouwstra KiaraGrouwstra merged commit 3a5ef15 into typed-typings:master Aug 15, 2017
@ikatyang ikatyang deleted the merge-ikatyang-types-ramda branch August 15, 2017 02:22
This was referenced Aug 15, 2017
@ikatyang
Copy link
Member Author

@tycho01 I've replaced the token on travis with my bot's token.

@KiaraGrouwstra
Copy link
Member

cool, I've ditched the token; thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants